You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fix both storage and postgrest to not mutate external httpx client, given it is shared, by instead keeping their own copy of their parameters to pass into the request call. In order to properly manipulate URLs without mutation, I've decided to add the yarl dependency, which returns a new object upon path manipulation.
For the postgrest library, I've also decided to factor out all of the parameters that go into a request in a RequestConfig object, for it is very easy to miss a spot otherwise. I've had to change the tests to account for this. However, I think this greatly simplifies most of the libraries, as 99% of the code does not need to be async/sync, and can be stated only once in a further rewrite.
I do think letting the httpx client be passed in was a mistake in general, for it adds way too much ambiguity on how parameters are merged. For instance, base_url should be entirely ignored if set in httpx, while the other parameters should be additive in general.
I do think letting the httpx client be passed in was a mistake in general, for it adds way too much ambiguity on how parameters are merged. For instance, base_url should be entirely ignored if set in httpx, while the other parameters should be additive in general.
It was either this or passing every parameter that httpx expects as a config parameter which felt like duplication work. Let's discuss in a new issue if you have another approach you think would work better.
I will pull this code down and test against my local codebase.
Thank you, please let me know of the results.
It was either this or passing every parameter that httpx expects as a config parameter which felt like duplication work.
The problem is that it is still happening, headers and base_url are such examples of arguments that can be set on both, and are ambiguous as to what setting both means. Furthermore, RequestConfig is basically a configuration class for httpx, which we might decide to just use directly instead. Setting things on it is much easier as we can assign immutable semantics, instead of httpx's mutable API, which is horrible for sharing configuration. I believe the only way forward is to expand something like RequestConfig into all libraries, and pass it around with immutable semantics, ditching the httpx client argument.
Just noticed that this problem is not only on these two libraries, functions also has it. realtime does not accept an httpx client, while auth accepts it but does store its own url and headers.
o-santi
changed the title
fix: do not mutate httpx client inside storage and postgrest
fix: do not mutate httpx client inside storage, postgrest and functions
Oct 7, 2025
The reason will be displayed to describe this comment to others. Learn more.
Async/SyncBucketActionsMixin class is missing the base_url and only has the path as the URL hence the error above. This needs to be updated to match the Async/SyncStorageBucketAPI use of base_url.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
o-santi
changed the title
What kind of change does this PR introduce?
Fix both
storageandpostgrestto not mutate external httpx client, given it is shared, by instead keeping their own copy of their parameters to pass into the request call. In order to properly manipulate URLs without mutation, I've decided to add theyarldependency, which returns a new object upon path manipulation.For the
postgrestlibrary, I've also decided to factor out all of the parameters that go into a request in aRequestConfigobject, for it is very easy to miss a spot otherwise. I've had to change the tests to account for this. However, I think this greatly simplifies most of the libraries, as 99% of the code does not need to be async/sync, and can be stated only once in a further rewrite.Additional context
Should fix #1245.